[refactor] #85 랜덤포즈 중복 확인 위치 변경#88
Merged
Merged
Conversation
기존의 단일 랜덤 포즈를 가져오는 `getRandomPose` 함수를, 중복을 제외하고 여러 개의 포즈를 가져올 수 있도록 `getSingleRandomPose` 와 `getMultipleRandomPose` 함수로 분리했습니다. - `excludeIds` 파라미터를 추가하여 이미 가져온 포즈 ID를 제외하고 새로운 포즈를 요청합니다. - `maxRetry` 파라미터를 통해 재시도 횟수를 제한하고, 새로운 포즈를 찾지 못할 경우 `RandomPoseRetryExhaustedException`을 발생시킵니다.
ViewModel에서 랜덤 포즈를 호출하고, 중복을 직접 검사하던 로직을 Repository 계층으로 옮겼습니다. - `PoseRepository`에 중복을 제외하고 여러 포즈를 가져오는 `getMultipleRandomPose`와 단일 포즈를 가져오는 `getSingleRandomPose`를 추가했습니다. - ViewModel은 이 새로운 Repository 메서드를 호출하여 비즈니스 로직을 단순화했습니다. - 중복 포즈를 가져오려다 재시도 횟수를 모두 소진하면 발생하는 `RandomPoseRetryExhaustedException`을 새로 정의하여 예외 처리를 명확히 했습니다. - `MAXIMUM_RANDOM_POSE_FALLBACK_COUNT` 상수의 이름을 `MAXIMUM_RANDOM_POSE_RETRY_COUNT`로 변경하여 역할을 더 명확하게 표현했습니다.
Walkthrough이 PR은 무작위 포즈 중복 확인 로직을 ViewModel에서 Repository 계층으로 이동시킵니다. 새로운 Repository 메서드 두 개를 추가하여 재시도 지원 및 중복 필터링을 구현하고, 재시도 소진 시 발생시킬 새로운 예외를 도입합니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/PoseRepositoryImpl.kt`:
- Around line 83-104: getMultipleRandomPose currently returns a partial list
when maxRetry is reached before collecting poseSize items; update the method
(getMultipleRandomPose) to treat partial results as failure by throwing
RandomPoseRetryExhaustedException when result.size < poseSize (not just when
empty), or alternatively add a clear comment and change the caller
(fetchInitialPoses) to handle partial lists—prefer throwing the same
RandomPoseRetryExhaustedException with a descriptive message so callers always
receive either a full list of size poseSize or an exception.
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt`:
- Around line 192-198: The onFailure block in RandomPoseViewModel.kt currently
logs the same error twice; remove the first unconditional Timber.e(error) inside
the onFailure lambda and keep the conditional logging that follows (i.e., retain
reduce { copy(isLoading = false) } and then log either Timber.e(error, "중복 포즈")
when error is RandomPoseRetryExhaustedException or Timber.e(error) otherwise) so
the error is only emitted once.
- Around line 155-161: Prefetched poses added in RandomPoseViewModel (inside the
getSingleRandomPose onSuccess block) are appended only to poseList and not to
committedScraps, causing handleScrapToggle to early-return when committedScrap
== null; update the reducer to also add an entry to committedScraps for the new
pose(s) (use the pose's id) — e.g., merge state.committedScraps with a mapping
for pose.id to the pose.committedScrap value (or false/default) when calling
reduce so prefetched items have a non-null committedScrap and handleScrapToggle
will work.
🧹 Nitpick comments (2)
core/data-api/src/main/java/com/neki/android/core/dataapi/repository/PoseRepository.kt (1)
22-33:maxRetry가 Repository 인터페이스에 노출되어 있습니다.
maxRetry는 재시도 구현 세부사항으로, Repository 인터페이스보다는 구현체 내부에서 관리하는 것이 더 적절할 수 있습니다. 현재 구조에서는 호출자(ViewModel)가PoseConst를 통해 값을 지정하고 있지만, 이는 레이어 간 결합도를 높입니다.Repository 구현체가 내부적으로 기본 재시도 횟수를 가지도록 하거나, 인터페이스에 기본값(
maxRetry: Int = DEFAULT_RETRY)을 두는 방안을 고려해 보세요.feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt (1)
152-167: Prefetch 시state.randomPoseIds캡처 시점을 확인하세요.
state는onIntent호출 시점의 스냅샷으로,reduce { copy(currentIndex = nextIndex) }(Line 149) 이전의 상태입니다.poseList는 변경되지 않으므로randomPoseIds에는 영향이 없지만, 동시에 여러 prefetch 코루틴이 실행되면 같은excludeIds를 사용하게 되어 중복 포즈가 추가될 수 있습니다.빠르게 스와이프하는 경우 여러 prefetch가 동시 실행될 수 있으므로, 최신 상태의
store.uiState.value.randomPoseIds를 사용하거나 동시 prefetch를 제한하는 것을 고려해 보세요.
Comment on lines
+83
to
104
| override suspend fun getMultipleRandomPose( | ||
| headCount: PeopleCount, | ||
| excludeIds: Set<Long>, | ||
| poseSize: Int, | ||
| maxRetry: Int, | ||
| ): Result<List<Pose>> = runSuspendCatching { | ||
| val result = mutableListOf<Pose>() | ||
| val collectedIds = excludeIds.toMutableSet() | ||
| var retryCount = 0 | ||
|
|
||
| while (result.size < poseSize && retryCount < maxRetry) { | ||
| val pose = poseService.getRandomPose(headCount = headCount.name).data.toModel() | ||
| if (pose.id !in collectedIds) { | ||
| result.add(pose) | ||
| collectedIds.add(pose.id) | ||
| } else { | ||
| retryCount++ | ||
| } | ||
| } | ||
|
|
||
| result.ifEmpty { throw RandomPoseRetryExhaustedException("새로운 포즈를 찾지 못했어요") } | ||
| } |
There was a problem hiding this comment.
getMultipleRandomPose가 poseSize보다 적은 결과를 조용히 반환할 수 있습니다.
retryCount가 maxRetry에 도달하면 while 루프가 종료되지만, result가 비어있지 않으면 (1개 이상) 예외 없이 부분 결과를 반환합니다. 예를 들어 poseSize=4, maxRetry=7일 때 2개만 수집하고 7회 중복이 발생하면 2개짜리 리스트가 반환됩니다.
호출부(fetchInitialPoses)에서는 반환된 리스트를 그대로 poseList에 설정하므로, 요청한 개수보다 적은 포즈가 표시될 수 있습니다.
의도된 동작이라면 주석으로 명시하고, 그렇지 않다면 result.size < poseSize일 때도 예외를 던지거나 호출부에서 처리하는 것을 권장합니다.
💡 부분 결과도 예외로 처리하는 경우
- result.ifEmpty { throw RandomPoseRetryExhaustedException("새로운 포즈를 찾지 못했어요") }
+ if (result.size < poseSize) {
+ throw RandomPoseRetryExhaustedException("요청한 ${poseSize}개 중 ${result.size}개만 찾았어요")
+ }
+ result🤖 Prompt for AI Agents
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/PoseRepositoryImpl.kt`
around lines 83 - 104, getMultipleRandomPose currently returns a partial list
when maxRetry is reached before collecting poseSize items; update the method
(getMultipleRandomPose) to treat partial results as failure by throwing
RandomPoseRetryExhaustedException when result.size < poseSize (not just when
empty), or alternatively add a clear comment and change the caller
(fetchInitialPoses) to handle partial lists—prefer throwing the same
RandomPoseRetryExhaustedException with a descriptive message so callers always
receive either a full list of size poseSize or an exception.
Comment on lines
+155
to
+161
| poseRepository.getSingleRandomPose( | ||
| headCount = peopleCount, | ||
| excludeIds = state.randomPoseIds, | ||
| maxRetry = PoseConst.MAXIMUM_RANDOM_POSE_RETRY_COUNT, | ||
| ).onSuccess { pose -> | ||
| reduce { copy(poseList = (poseList + pose).toImmutableList()) } | ||
| }.onFailure { error -> |
There was a problem hiding this comment.
Prefetch로 추가된 포즈의 committedScraps가 갱신되지 않습니다.
Line 160에서 새 포즈를 poseList에만 추가하고 committedScraps에는 추가하지 않습니다. handleScrapToggle(Line 96)에서 committedScrap == null이면 조기 반환하므로, prefetch된 포즈에 대한 스크랩 토글이 서버에 반영되지 않습니다.
🐛 committedScraps 업데이트 추가
).onSuccess { pose ->
- reduce { copy(poseList = (poseList + pose).toImmutableList()) }
+ reduce {
+ copy(
+ poseList = (poseList + pose).toImmutableList(),
+ committedScraps = committedScraps + (pose.id to pose.isScrapped),
+ )
+ }
}.onFailure { error ->📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| poseRepository.getSingleRandomPose( | |
| headCount = peopleCount, | |
| excludeIds = state.randomPoseIds, | |
| maxRetry = PoseConst.MAXIMUM_RANDOM_POSE_RETRY_COUNT, | |
| ).onSuccess { pose -> | |
| reduce { copy(poseList = (poseList + pose).toImmutableList()) } | |
| }.onFailure { error -> | |
| poseRepository.getSingleRandomPose( | |
| headCount = peopleCount, | |
| excludeIds = state.randomPoseIds, | |
| maxRetry = PoseConst.MAXIMUM_RANDOM_POSE_RETRY_COUNT, | |
| ).onSuccess { pose -> | |
| reduce { | |
| copy( | |
| poseList = (poseList + pose).toImmutableList(), | |
| committedScraps = committedScraps + (pose.id to pose.isScrapped), | |
| ) | |
| } | |
| }.onFailure { error -> |
🤖 Prompt for AI Agents
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt`
around lines 155 - 161, Prefetched poses added in RandomPoseViewModel (inside
the getSingleRandomPose onSuccess block) are appended only to poseList and not
to committedScraps, causing handleScrapToggle to early-return when
committedScrap == null; update the reducer to also add an entry to
committedScraps for the new pose(s) (use the pose's id) — e.g., merge
state.committedScraps with a mapping for pose.id to the pose.committedScrap
value (or false/default) when calling reduce so prefetched items have a non-null
committedScrap and handleScrapToggle will work.
Comment on lines
+192
to
198
| }.onFailure { error -> | ||
| Timber.e(error) | ||
| reduce { copy(isLoading = false) } | ||
| postSideEffect(RandomPoseEffect.ShowToast("포즈를 불러오는데 실패했어요")) | ||
| if (error is RandomPoseRetryExhaustedException) | ||
| Timber.e(error, "중복 포즈") | ||
| else Timber.e(error) | ||
| } |
There was a problem hiding this comment.
onFailure에서 에러가 이중 로깅됩니다.
Line 193에서 Timber.e(error)로 한 번 로깅한 후, Line 195-197에서 동일한 에러를 다시 로깅합니다. 모든 실패 케이스에서 에러 메시지가 두 번 출력됩니다.
🐛 중복 로깅 수정
}.onFailure { error ->
- Timber.e(error)
reduce { copy(isLoading = false) }
if (error is RandomPoseRetryExhaustedException)
Timber.e(error, "중복 포즈")
else Timber.e(error)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| }.onFailure { error -> | |
| Timber.e(error) | |
| reduce { copy(isLoading = false) } | |
| postSideEffect(RandomPoseEffect.ShowToast("포즈를 불러오는데 실패했어요")) | |
| if (error is RandomPoseRetryExhaustedException) | |
| Timber.e(error, "중복 포즈") | |
| else Timber.e(error) | |
| } | |
| }.onFailure { error -> | |
| reduce { copy(isLoading = false) } | |
| if (error is RandomPoseRetryExhaustedException) | |
| Timber.e(error, "중복 포즈") | |
| else Timber.e(error) | |
| } |
🤖 Prompt for AI Agents
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt`
around lines 192 - 198, The onFailure block in RandomPoseViewModel.kt currently
logs the same error twice; remove the first unconditional Timber.e(error) inside
the onFailure lambda and keep the conditional logging that follows (i.e., retain
reduce { copy(isLoading = false) } and then log either Timber.e(error, "중복 포즈")
when error is RandomPoseRetryExhaustedException or Timber.e(error) otherwise) so
the error is only emitted once.
Ojongseok
approved these changes
Feb 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 관련 이슈
📙 작업 설명
랜덤 포즈 중복 체크 로직을 ViewModel에서 Repository로 이전
getSingleRandomPose/getMultipleRandomPoseRepository 함수 추가RandomPoseRetryExhaustedException커스텀 예외 추가ViewModel의
fetchRandomPose및FetchPoseResultsealed class 제거MAXIMUM_RANDOM_POSE_FALLBACK_COUNT→MAXIMUM_RANDOM_POSE_RETRY_COUNT네이밍 개선develop 으로 base설정했습니다.
📷 스크린샷
Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링